Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Dec 28, 2025

  • Explore repository structure and understand the codebase
  • Identify the root cause of Windows tray icon issues
  • Fix Windows tray implementation to use thread-safe callbacks
  • Refactor based on feedback: wrap callbacks at call site instead of passing tkinter_root
  • Simplify macOS tray implementation by removing redundant thread marshalling
  • Improve documentation based on code review
  • Run final code review
  • Run security scan
Original prompt

On windows closing the application and the tray icon isnt working well. clicking quit in the tray menu does nothing and i cant get the window back with show window


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI and others added 4 commits December 28, 2025 19:55
- Pass tkinter_root to Windows tray implementation
- Schedule all callbacks on main Tkinter thread using root.after()
- Fixes issue where quit and show window didn't work from tray menu
- Ensures thread-safe UI updates from tray icon events

Co-authored-by: bryfur <7673964+bryfur@users.noreply.github.com>
Co-authored-by: bryfur <7673964+bryfur@users.noreply.github.com>
- Extract common pattern into _schedule_callback helper method
- Improves code maintainability and readability

Co-authored-by: bryfur <7673964+bryfur@users.noreply.github.com>
Co-authored-by: bryfur <7673964+bryfur@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix tray icon quit functionality on Windows Fix Windows tray icon callbacks failing due to thread safety issues Dec 28, 2025
Copilot AI requested a review from bryfur December 28, 2025 20:01
@bryfur
Copy link
Owner

bryfur commented Dec 28, 2025

@copilot I dont like passing the tkinter root in, should the callbacks just all pass in wrapped in tkinter after, ensure all tray implementations would work correctly with that setup

… tray implementations

- Remove tkinter_root parameter from TrayIcon and WindowsTrayImpl
- Wrap all callbacks with lambda: root.after(0, callback) in MainWindow._setup_tray()
- This keeps tray implementations simpler and platform-agnostic
- Thread safety is now handled at the callback registration point

Co-authored-by: bryfur <7673964+bryfur@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Dec 28, 2025

@copilot I dont like passing the tkinter root in, should the callbacks just all pass in wrapped in tkinter after, ensure all tray implementations would work correctly with that setup

Done! I've refactored the solution to wrap callbacks with root.after() at the call site instead of passing tkinter_root to the tray implementations. This keeps the tray implementations simpler and platform-agnostic.

The callbacks are now wrapped in MainWindow._setup_tray():

on_show=lambda: self.root.after(0, self._show_window),
on_quit=lambda: self.root.after(0, self.quit_app),
# etc.

This approach ensures thread safety while maintaining clean separation of concerns. Commit: 9eeba2e

@bryfur
Copy link
Owner

bryfur commented Dec 28, 2025

@copilot can we simplify the macos tray now that callbacks are always on the main loop

Copilot AI and others added 2 commits December 28, 2025 20:35
…alling

- Remove performSelectorOnMainThread calls since callbacks are already wrapped with root.after()
- Call setupStatusBar and updateMonitorMenuTitle_ directly - already on main thread via async_mainloop
- Reduce setup delay from 0.3s to 0.1s
- Update documentation to reflect simplified implementation

Co-authored-by: bryfur <7673964+bryfur@users.noreply.github.com>
Co-authored-by: bryfur <7673964+bryfur@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants